Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve cstring cache internals #4780

Merged
merged 4 commits into from
Jul 4, 2024
Merged

Improve cstring cache internals #4780

merged 4 commits into from
Jul 4, 2024

Conversation

asl
Copy link
Contributor

@asl asl commented Jul 3, 2024

  • Add cache introspection methods (is_cached / get_cached) without implicitly constructing cstrings
  • Refactor caching to use only single map lookup regardless whether we'd need to insert something into cache or the value is already there
  • Switch to better underlying map

This is a spin-off from #4774

asl added 2 commits July 3, 2024 11:56
@asl
Switch to abseil node_hash_set for cstring internal hash. Reduce numb…
3ff09f5 
…er of lookups while there.

Signed-off-by: Anton Korobeynikov <[email protected]>
@asl
Import get_cached method
272cbab 
Signed-off-by: Anton Korobeynikov <[email protected]>
@asl asl requested a review from vlstill July 3, 2024 19:23
@asl asl mentioned this pull request Jul 3, 2024
Merged
@asl
Usual "unbreak Bazel" commit
1947b52 
Signed-off-by: Anton Korobeynikov <[email protected]>
@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Jul 3, 2024
@fruffy fruffy requested a review from ChrisDodd July 3, 2024 19:50
vlstill
vlstill approved these changes Jul 4, 2024
View reviewed changes
Copy link
Contributor

@vlstill vlstill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Just maybe use [[maybe_unused]] instead of the void cast.

test/gtest/cstring.cpp Outdated Show resolved Hide resolved
test/gtest/cstring.cpp Outdated Show resolved Hide resolved
asl
asl commented Jul 4, 2024
View reviewed changes
test/gtest/cstring.cpp Outdated Show resolved Hide resolved
asl
asl commented Jul 4, 2024
View reviewed changes
test/gtest/cstring.cpp Outdated Show resolved Hide resolved
@asl @vlstill
Apply suggestions from code review
35efadd 
Co-authored-by: Vladimír Štill <[email protected]>
Signed-off-by: Anton Korobeynikov <[email protected]>
@asl asl added this pull request to the merge queue Jul 4, 2024
Merged via the queue into p4lang:main with commit db050c6 Jul 4, 2024
17 checks passed
@asl asl deleted the cstring-cache branch July 4, 2024 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisDodd ChrisDodd ChrisDodd approved these changes

@vlstill vlstill vlstill approved these changes

Assignees
No one assigned
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@asl @vlstill @ChrisDodd @fruffy